fix(umg): bind_on_hovered and bind_on_value_changed author real bound-event nodes#482
Conversation
…-event nodes
bind_on_hovered and bind_on_value_changed returned an instruction string ("Bind '...' to ...'s
... event.") instead of authoring a node, unlike bind_on_clicked which creates a real
UK2Node_ComponentBoundEvent.
Extracted the working bind_on_clicked path into a shared BindComponentDelegateEvent helper and
routed both actions through it:
- bind_on_hovered -> Button OnHovered.
- bind_on_value_changed -> per widget type: Slider/SpinBox OnValueChanged, CheckBox
OnCheckStateChanged, ComboBoxString OnSelectionChanged; unsupported widgets return
UNSUPPORTED_WIDGET instead of a misleading success.
Both now return a real node (nodeId + compileSucceeded), are idempotent (reuse an existing bound
event), and only dirty/recompile when something changed. create_property_binding is left unchanged.
Verified live in UE 5.8: bind_on_hovered and bind_on_value_changed (Slider) both produce real
K2Node_ComponentBoundEvent nodes with compileSucceeded:true.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughA shared helper now creates or reuses component-bound event nodes for widget delegate bindings, conditionally recompiling only when the blueprint structure changes. The clicked, hovered, and value-changed widget handlers now route through that helper, including widget-type-based delegate selection for value changes. ChangesWidget Event Binding Refactor
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp (1)
25-116: ⚡ Quick winRoute
bind_on_clickedthrough the helper as well.The new helper duplicates the full
OnClickedbinding path, butbind_on_clickedstill keeps its own copy. That means fixes like the structural-dirtying issue above must be patched twice. The helper signature already supportsButtonWidget,UButton::StaticClass(), andOnClicked, so the clicked handler can delegate after validation.Also applies to: 159-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp` around lines 25 - 116, The bind_on_clicked handler is duplicating the full binding logic instead of using the new BindComponentDelegateEvent helper function that was created. Locate the bind_on_clicked handler implementation and refactor it to call BindComponentDelegateEvent with the appropriate parameters (TargetWidget as ButtonWidget, DelegateOwnerClass as UButton::StaticClass(), DelegateName as OnClicked, and other metadata) instead of implementing the same logic directly. This eliminates code duplication and ensures that any future fixes to the binding process only need to be applied once in the helper function.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp`:
- Around line 85-100: When creating a new UK2Node_ComponentBoundEvent node in
the EventGraph, the code marks the Blueprint as modified via
MarkBlueprintAsModified but fails to mark it as structurally modified. Graph
node creation is a structural change to the Blueprint, so you need to add a call
to MarkBlueprintAsStructurallyModified in addition to MarkBlueprintAsModified.
Specifically, when bCreatedNew is true (indicating a new BoundNode was created),
call FBlueprintEditorUtils::MarkBlueprintAsStructurallyModified on the WidgetBP
before or after the MarkBlueprintAsModified call to properly notify the
Blueprint system of the structural change.
---
Nitpick comments:
In
`@plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp`:
- Around line 25-116: The bind_on_clicked handler is duplicating the full
binding logic instead of using the new BindComponentDelegateEvent helper
function that was created. Locate the bind_on_clicked handler implementation and
refactor it to call BindComponentDelegateEvent with the appropriate parameters
(TargetWidget as ButtonWidget, DelegateOwnerClass as UButton::StaticClass(),
DelegateName as OnClicked, and other metadata) instead of implementing the same
logic directly. This eliminates code duplication and ensures that any future
fixes to the binding process only need to be applied once in the helper
function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7deb5908-9bba-4d14-95af-530dde7d3ab7
📒 Files selected for processing (1)
plugins/McpAutomationBridge/Source/McpAutomationBridge/Private/Domains/WidgetAuthoring/Bindings/McpAutomationBridge_WidgetAuthoringEventBindings.cpp
|
On adding |
…elper Review follow-up, both findings: - bind_on_clicked now delegates to BindComponentDelegateEvent after its Button lookup, removing the duplicated binding path so future fixes land in one place. Response contract is unchanged (same fields, same "OnClicked event bound" message). - BindComponentDelegateEvent marks the Blueprint structurally modified when it creates a new bound-event node — the same call the Designer's FKismetEditorUtilities::CreateNewBoundEventForComponent path makes — which now covers all three bind actions.
|
Both review findings addressed and pushed:
Live-verified in a UE 5.8 editor on a fresh widget blueprint: |
2 participants
Problem
bind_on_hoveredandbind_on_value_changedreturned a plain instruction string (e.g."Bind 'OnButtonHovered' to BtnStart's OnHovered event.") instead of authoring anything — no node, nonodeId. Onlybind_on_clickedcreated a realUK2Node_ComponentBoundEvent.Fix (
EventBindings.cpp)Extracted the working
bind_on_clickedimplementation into a sharedBindComponentDelegateEventhelper and routed the other two through it:bind_on_hovered→ ButtonOnHovered.bind_on_value_changed→ per widget type: Slider/SpinBoxOnValueChanged, CheckBoxOnCheckStateChanged, ComboBoxStringOnSelectionChanged; unsupported widgets returnUNSUPPORTED_WIDGETrather than a misleading success.Both now author a real
UK2Node_ComponentBoundEvent(returnsnodeId+compileSucceeded), are idempotent (reuse an existing bound event viaFindBoundEventForComponent), and only dirty/recompile when something actually changed — matchingbind_on_clickedexactly.create_property_bindingis intentionally left unchanged (separate follow-up).Validation (UE 5.8, live)
bind_on_hovered(Button) andbind_on_value_changed(Slider) both produce realK2Node_ComponentBoundEventnodes (OnButtonHoverEvent/OnFloatValueChangedEvent) withcompileSucceeded:true.🤖 Generated with Claude Code